Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Provide ability to add labels to Cluster object #809

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Provide ability to add labels to Cluster object #809

merged 1 commit into from
Jan 13, 2025

Conversation

wahabmk
Copy link
Contributor

@wahabmk wahabmk commented Dec 18, 2024 •
edited
Loading

Description

Provide ability to add labels to the Cluster object created when installing a clusterTemplate by:

  1. Adding .Values.ClusterLabels to all clusterTemplate helm charts.
  2. Using ManagedCluster's own labels via the HMC controller if none have been defined in .Values.ClusterLabels.

Testing

As can be seen from the output below, the .metadata.labels from ManagedCluster have been added to the Cluster object:

➜  ~ kubectl -n hmc-system get managedclusters.hmc.mirantis.com wali-dev-1 --show-labels
NAME         READY   STATUS                    LABELS
wali-dev-1   True    ManagedCluster is ready   labelA=A,labelB=B
➜  ~
➜  ~
➜  ~ kubectl -n hmc-system get cluster --show-labels                                
NAME         CLUSTERCLASS   PHASE         AGE   VERSION   LABELS
wali-dev-1                  Provisioned   13m             app.kubernetes.io/managed-by=Helm,helm.toolkit.fluxcd.io/name=wali-dev-1,helm.toolkit.fluxcd.io/namespace=hmc-system,labelA=A,labelB=B,sveltos-agent=present

@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch 2 times, most recently from 805f843 to f080795 Compare December 18, 2024 18:12
@wahabmk wahabmk changed the title WIP Provide ability to add labels to Cluster object Dec 18, 2024
@wahabmk wahabmk self-assigned this Dec 18, 2024
@wahabmk wahabmk linked an issue Dec 18, 2024 that may be closed by this pull request
Set labels on Cluster object via ManagedCluster #801
Closed
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch from f080795 to 3475d9c Compare December 18, 2024 18:23
@wahabmk wahabmk marked this pull request as ready for review December 18, 2024 18:42
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch from 3475d9c to 44b336c Compare December 18, 2024 23:09
zerospiel
zerospiel reviewed Dec 19, 2024
View reviewed changes
Copy link
Contributor

@zerospiel zerospiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, 2 charts-related-mostly-nit comments

templates/cluster/vsphere-standalone-cp/templates/cluster.yaml Outdated Show resolved Hide resolved
templates/cluster/vsphere-standalone-cp/values.yaml Outdated Show resolved Hide resolved
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch 2 times, most recently from 651f907 to b5a7452 Compare December 19, 2024 21:48
@wahabmk wahabmk requested a review from zerospiel December 19, 2024 22:39
zerospiel
zerospiel previously approved these changes Dec 20, 2024
View reviewed changes
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch from b5a7452 to 1f07334 Compare December 20, 2024 14:34
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch from 1f07334 to 9396b6e Compare December 27, 2024 14:51
@wahabmk wahabmk requested a review from Kshatrix December 30, 2024 12:59
@DinaBelova DinaBelova requested a review from zerospiel January 7, 2025 20:48
zerospiel
zerospiel previously approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@zerospiel zerospiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the PR needs to be rebased and @Kshatrix should also check whether it's okay to assign labels to the CAPI clusters

@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch 2 times, most recently from c514276 to c475159 Compare January 8, 2025 16:19
gmlexx
gmlexx previously approved these changes Jan 9, 2025
View reviewed changes
@wahabmk wahabmk dismissed stale reviews from gmlexx and zerospiel via 7d61207 January 13, 2025 11:04
@wahabmk wahabmk force-pushed the custom-labels-managed-cluster branch from c475159 to 7d61207 Compare January 13, 2025 11:04
@mestadler
Copy link

Good to me!

@wahabmk wahabmk requested a review from zerospiel January 13, 2025 14:17
@zerospiel
Copy link
Contributor

LGTM, @Kshatrix could you check whether you've reconciled on the approach and if the initial is okay, then I'm ready to approve and merge the PR

@wahabmk
Copy link
Contributor Author

wahabmk commented Jan 13, 2025

LGTM, @Kshatrix could you check whether you've reconciled on the approach and if the initial is okay, then I'm ready to approve and merge the PR

In our last meeting @Kshatrix mentioned he didn't block the PR by requesting any changes on the PR so we can merge it

@zerospiel zerospiel merged commit 2b9d92c into k0rdent:main Jan 13, 2025
5 checks passed
@wahabmk wahabmk deleted the custom-labels-managed-cluster branch January 13, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zerospiel zerospiel zerospiel approved these changes

@gmlexx gmlexx gmlexx left review comments

@eromanova eromanova Awaiting requested review from eromanova eromanova is a code owner

@a13x5 a13x5 Awaiting requested review from a13x5 a13x5 is a code owner

@kylewuolle kylewuolle Awaiting requested review from kylewuolle

@BROngineer BROngineer Awaiting requested review from BROngineer

@Kshatrix Kshatrix Awaiting requested review from Kshatrix Kshatrix is a code owner

Assignees

@wahabmk wahabmk

Labels
None yet
Projects
k0rdent
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Set labels on Cluster object via ManagedCluster
5 participants
@wahabmk @mestadler @zerospiel @gmlexx @Kshatrix